drivers: mfd: add support for MAX2221X multi-function device #97584
Conversation
|
Hello @csarip, and thank you very much for your first pull request to the Zephyr project! |
zephyrbot
added
area: PWM
zephyrbot
requested review from
MaureenHelm,
aasinclair,
anangl,
galak,
henrikbrixandersen,
microbuilder,
nashif,
nordic-auko,
ozersa,
seov-nordic,
ttmut and
yasinustunerg
drivers/pwm/Kconfig.max2221x
Outdated
| #if PWM_MAX2221X | ||
|
|
||
| config PWM_MAX2221X_INIT_PRIORITY | ||
| int "Init priority" | ||
| default 80 | ||
| help | ||
| Analog Devices MAX22216/MAX22217 PWM driver initialization priority. | ||
|
|
||
| #endif #PWM_MAX2221X |
There was a problem hiding this comment.
Is there a reason not to use PWM_INIT_PRIORITY?
If you do need a driver-specific init priority, you need to uncomment the if/endif statements and convert config PWM_MAX2221X to a menuconfig
There was a problem hiding this comment.
Is there a reason not to use
PWM_INIT_PRIORITY?If you do need a driver-specific init priority, you need to uncomment the if/endif statements and convert
config PWM_MAX2221Xto a menuconfig
Thank you for the suggestion. I’ve converted it to a menuconfig option instead. I had to assign a different initialization priority value to the PWM to ensure that its dependency, the MFD, initializes first. By default, CONFIG_MFD_INIT_PRIORITY is 80, while CONFIG_PWM_INIT_PRIORITY is 50, which causes the PWM to initialize before its dependency.
Implements SPI-based register access (read, write, update) for the MAX22216/MAX22217 multi-function device. Signed-off-by: Cherrence Sarip <[email protected]>
csarip
force-pushed
the
upstream/dev-max2221x
branch
from
980e049 to
7a1986f
Compare
Add MISC driver support for the MAX22216/MAX22217 device. The driver provides miscellaneous functions and uses the parent MFD device for register access. Signed-off-by: John Erasmus Mari Geronimo <[email protected]> Signed-off-by: Cherrence Sarip <[email protected]>
Implements PWM driver support for the MAX22216/MAX22217 device. This driver integrates with the Zephyr PWM API and uses the parent MFD device to perform register access. Signed-off-by: Cherrence Sarip <[email protected]>
csarip
force-pushed
the
upstream/dev-max2221x
branch
from
7a1986f to
be50311
Compare
|
v2:
|
|
|
Please check the issues found by SonarQube cloud analysis as well. Especially the warnings about misleading
|
| int max2221x_enable_channel(const struct device *dev, uint8_t channel) | ||
| { | ||
| const struct misc_max2221x_config *config = dev->config; | ||
|
|
||
| if (channel >= MAX2221X_NUM_CHANNELS) { | ||
| LOG_ERR("Invalid channel"); | ||
| return -EINVAL; | ||
| } | ||
|
|
||
| return max2221x_reg_update(config->parent, MAX2221X_REG_GLOBAL_CTRL, | ||
| MAX2221X_CNTL0_MASK << channel, 1); | ||
| } | ||
|
|
||
| int max2221x_disable_channel(const struct device *dev, uint8_t channel) | ||
| { | ||
| const struct misc_max2221x_config *config = dev->config; | ||
|
|
||
| if (!dev) { | ||
| printk("Error: Device pointer is NULL\n"); | ||
| return -EINVAL; | ||
| } | ||
|
|
||
| if (channel >= MAX2221X_NUM_CHANNELS) { | ||
| LOG_ERR("Invalid channel"); | ||
| return -EINVAL; | ||
| } | ||
|
|
||
| return max2221x_reg_update(config->parent, MAX2221X_REG_GLOBAL_CTRL, | ||
| MAX2221X_CNTL0_MASK << channel, 0); | ||
| } |
There was a problem hiding this comment.
As far as I see, there are many code repetitions in the source code. We may eliminate these functions into single function.
| int max2221x_enable_channel(const struct device *dev, uint8_t channel) | |
| { | |
| const struct misc_max2221x_config *config = dev->config; | |
| if (channel >= MAX2221X_NUM_CHANNELS) { | |
| LOG_ERR("Invalid channel"); | |
| return -EINVAL; | |
| } | |
| return max2221x_reg_update(config->parent, MAX2221X_REG_GLOBAL_CTRL, | |
| MAX2221X_CNTL0_MASK << channel, 1); | |
| } | |
| int max2221x_disable_channel(const struct device *dev, uint8_t channel) | |
| { | |
| const struct misc_max2221x_config *config = dev->config; | |
| if (!dev) { | |
| printk("Error: Device pointer is NULL\n"); | |
| return -EINVAL; | |
| } | |
| if (channel >= MAX2221X_NUM_CHANNELS) { | |
| LOG_ERR("Invalid channel"); | |
| return -EINVAL; | |
| } | |
| return max2221x_reg_update(config->parent, MAX2221X_REG_GLOBAL_CTRL, | |
| MAX2221X_CNTL0_MASK << channel, 0); | |
| } | |
| static int max2221x_set_channel_state(const struct device *dev, uint8_t channel, bool enable) | |
| { | |
| if (!dev) { | |
| printk("Error: Device pointer is NULL\n"); | |
| return -EINVAL; | |
| } | |
| const struct misc_max2221x_config *config = dev->config; | |
| if (channel >= MAX2221X_NUM_CHANNELS) { | |
| LOG_ERR("Invalid channel"); | |
| return -EINVAL; | |
| } | |
| return max2221x_reg_update(config->parent, MAX2221X_REG_GLOBAL_CTRL, | |
| MAX2221X_CNTL0_MASK << channel, | |
| enable ? 1 : 0); | |
| } |
| drivers.pwm.max2221x.build: | ||
| platform_allow: nucleo_l476rg |
There was a problem hiding this comment.
Need to add
extra_args: DTC_OVERLAY_FILE=max2221x.overlay5 participants
Add initial support for the MAX22216/MAX22217 device. These devices have four 36 V half-bridges that support various drive modes, supported by MFD, PWM, and MISC drivers.